A new PHP JIT implementation based on IR JIT framework #12079

dstogov · 2023-08-29T20:14:41Z

This PR provides a new JIT implementation based on IR - Lightweight
JIT Compilation Framework.

Despite of the PHP 8.* JIT approach, that generates native code directly from
PHP byte-code, this implementation generates intermediate representation (IR)
and delegates all lower-level tasks to the IR Framework. IR for JIT is like an
AST for compiler.

Key benefits of the new JIT implementation:

Usage of IR opens possibilities for better optimization and register
allocation (the resulting native code is more efficient)
PHP doesn't have to care about most low-level details (different CPUs,
calling conventions, TLS details, etc)
it's much easier to implement support for new targets (e.g. RISCV)
IR framework is going to be developed separately from PHP and may accept
contributions from other projects (new optimizations, improvements, bug fixes)

Disadvantages:

JIT compilation becomes slower (this is almost invisible for tracing
JIT, but function JIT compilation of Wordpress becomes 4 times slower)

The necessary part of the IR Framework is embedded into php-src. So, the PR doesn't introduce new dependencies.

The new JIT implementation successfully passes all CI workflows, including
nightly, but it's still not mature and may cause failures.
To reduce risks, this patch doesn't remove the old JIT implementation (that
is the same as PHP-8.3 JIT). It's possible to build PHP with the old JIT by
configuring with --disable-opcache-jit-ir.
In the future the old implementation should be removed.

ext/opcache/jit/README-IR.md

.gitmodules

run-tests.php

nielsdos

First of all thanks for working on this new IR JIT.
I have some small comments with the help of SA. I only made a quick look so far.
Probably most of these are just me misunderstanding the code though.

EDIT: github bugged out and put the comments at the wrong lines because of the large diff...
EDIT 2: actually, it looks fine if you click "Files Changed" in the menu above. It's just in the comments here that it shows at the wrong line.

ext/opcache/jit/zend_jit_ir.c

dstogov · 2023-09-11T20:39:07Z

@nielsdos thank you for the review. Only one of your comment was wrong, and the rest should be already fixed.

dstogov · 2023-09-12T08:57:13Z

I wonder if there would be an interest to be able to export/save the IR (CLI method or similar)?

It's already possible to get the textual representation of the IR through -d opcache.jit_debug bits. e.g.

-d opcache.jit_debug=0x1000000 - IR created by PHP
-d opcache.jit_debug=0x2000000 - IR after all optimizations

This text may be loaded and processed without PHP, but generated code can't be executed, because the text includes hardcoded addresses of PHP API and helper functions.

That could open the door to be able to generate other targets using IR (not possible now for various limitations but wasm could be a very nice candidate to begin with).

This is one of the purposes of the IR project and the reason to develop it separately from PHP.
Adding a new IR backend should be easier then PHP one.

bukka · 2023-09-12T12:49:20Z

I haven't checked it much but sounds really promising. Think V8 is doing something similar so seems like the way forward.

I have got just one objection and that's usage of git submodules? They are cool for initial development especially if done by one person but I think they just suck if used for the bigger project with more contributors and will require updates for everyone building PHP from source (small but unnecessary). I also think it should be consistent with other bundled libs that are just included. Also when you look to other projects like nodejs, it includes sources in the repo. As I said I understand that it's easier for development but it complicates things for everyone else. In case we really want to use git submodules, it should be discussed separately and it will likely require RFC because I don't think there will be any agreement.

dstogov · 2023-09-12T13:33:48Z

I haven't checked it much but sounds really promising. Think V8 is doing something similar so seems like the way forward.

Yes. V8 uses very similar IR and compilation pipe-line.

I have got just one objection and that's usage of git submodules? They are cool for initial development especially if done by one person but I think they just suck if used for the bigger project with more contributors and will require updates for everyone building PHP from source (small but unnecessary). I also think it should be consistent with other bundled libs that are just included. Also when you look to other projects like nodejs, it includes sources in the repo. As I said I understand that it's easier for development but it complicates things for everyone else. In case we really want to use git submodules, it should be discussed separately and it will likely require RFC because I don't think there will be any agreement.

I completely agree, usage of submodules should be the common decision and they definitely will make troubles.
It's easier to embed the necessary files.

@derickr do you manage libdate updates with some tools or manually?

homersimpsons · 2023-09-13T16:13:58Z

Would it make sense to re-use an existing JIT compiler project such as https://github.com/bytecodealliance/wasmtime/tree/main/cranelift over re-creating our own dedicated to php ?

I do not know a lot in this field but I think there are already some existing JIT compiler we could plug into.

dstogov · 2023-09-13T19:49:51Z

Would it make sense to re-use an existing JIT compiler project such as https://github.com/bytecodealliance/wasmtime/tree/main/cranelift over re-creating our own dedicated to php ?

I do not know a lot in this field but I think there are already some existing JIT compiler we could plug into.

I already wrote LLVM based JIT, analysed usage of mir, libfim and few other JIT libraries.

Finally, I came to decision of creating a new PHP independent IR JIT framework mostly based on ideas bothered from HotSpot and LuaJIT. Combination of the ideas and practical approach allowed to make it much simple and significantly faster (PHP function JIT produces 15M of optimized native code per second, LLVM based PHP JIT did the same work more than a minute). This framework may be used for other languages (I have plans to write a C parser) and at the same time implements support for many PHP VM specific things.

withinboredom

This is pretty awesome. I just came to see how the sausage is made, though I don't understand everything, I like it.

Random suggestion would be to add a couple of attributes to hint to JIT what to do:

#[Jit\Hot] or something to hint that we want it always JIT'd because we expect it to be called very often.
#[Jit\Never] might be a useful escape hatch from JIT-specific bugs until a bugfix can be released.

ext/opcache/config.w32

withinboredom · 2023-09-15T13:55:23Z

ext/opcache/jit/zend_jit.c

+							}
+						}
+					}
+					if (need_move) {


This if-statement is a bit hard to follow with the dangling else at the bottom. Might I suggest doing something like:

if(!need_move) { ra[i].flags |= ZREG_PHI; continue; } // do things in if-statement

It'd reduce some of the nesting and make it a bit easier to read.

I don't think so. Following if/else should be easier than less-structural continue.
Anyway, this is just a coding preference.

withinboredom · 2023-09-15T13:58:11Z

ext/opcache/jit/zend_jit.c

+						}
+					} else if (ra[src].ref) {
+						ra[src].flags |= ZREG_STORE;
+					}


Similar to below, I think we can add a continue here and flatten this loop out a bit.

withinboredom · 2023-09-15T13:59:43Z

ext/opcache/jit/zend_jit.c

+						if (!ra[src].ref) {
+							ra[i].flags |= ZREG_LOAD;
+						} else {
+							ra[i].flags |= ZREG_PI;
+						}


Maybe something like this would work:

ra[i].flags |= ra[src].ref ? ZREG_PI : ZREG_LOAD;

Yes. This probably makes sense.

withinboredom · 2023-09-15T14:01:26Z

ext/opcache/jit/zend_jit.c

+				int k, src;
+
+				if (phi->pi >= 0) {
+					src = phi->sources[0];


I'm still wrapping my head around this, but is it possible for src to be a non-existent index on ra?

No.
ra[] is an array of size ssa->vars_count.
src as a SSA variable index, that must be in range [0..ssa->vars_count-1].

withinboredom · 2023-09-15T14:07:22Z

ext/opcache/jit/zend_jit.c

+		/* Remove useless register allocation */
+		for (i = 0; i < ssa->vars_count; i++) {
+			if (ra[i].ref &&
+			    ((ra[i].flags & ZREG_LOAD) ||


From above, if ra[i].ref is true, then ra[i].flags will always be ZREG_LOAD, maybe? Actually, not always. Looks like it's conditional, but leaving this comment here for now.

The idea of this loop is avoiding register allocation for variables that are not used directly but have to be loaded and stored at the same memory location.

derickr · 2023-09-15T14:33:59Z

@derickr do you manage libdate updates with some tools or manually?

I merge it by hand. There are only very few changes, so I use Meld to visually compare and merge changes over.

…cause of spilling

IR commit: 8977307f4e96ee03847d7f2eb809b3080f9ed662

IR commit: a2f8452b3d35a756cba38924f5c51a48a7207494

IR commit: 399a38771393c202a741336643118991290b4b1b

IR commit: 86685504274b0c71d9985b3c926dccaca2cacf9b

IR commit: d0686408e20cd8c8640e37ed52ab81403a2383cb

IR commit: d72ae866e09d17e879378767aceb91d51894818c

IR commit: d60d92516dc5f89b93cdf1df7a54141e83226b07

dstogov requested a review from iluuu1994 as a code owner August 29, 2023 20:14

github-actions bot added Category: Build System Category: Engine Extension: opcache labels Aug 29, 2023

dstogov requested a review from TimWolla as a code owner August 29, 2023 21:17

dstogov marked this pull request as draft August 30, 2023 00:04

dstogov force-pushed the php-ir branch 8 times, most recently from 1271dae to 4c15e7f Compare September 5, 2023 22:11

dstogov marked this pull request as ready for review September 6, 2023 14:39

dstogov force-pushed the php-ir branch from 4dc6e58 to 853504f Compare September 8, 2023 15:29

shyim reviewed Sep 8, 2023

View reviewed changes

ext/opcache/jit/README-IR.md Outdated Show resolved Hide resolved

TimWolla reviewed Sep 11, 2023

View reviewed changes

.gitmodules Outdated Show resolved Hide resolved

divinity76 reviewed Sep 11, 2023

View reviewed changes

run-tests.php Outdated Show resolved Hide resolved

nielsdos reviewed Sep 11, 2023

View reviewed changes

stkeke mentioned this pull request Sep 12, 2023

Link to the compiled function to improve performance #12182

Open

dstogov force-pushed the php-ir branch from d3bf88e to 4dfff03 Compare September 14, 2023 17:30

withinboredom reviewed Sep 15, 2023

View reviewed changes

dstogov added 27 commits October 20, 2023 17:58

Update IR

54d40de

Update IR

e7c04ec

Update IR

16b82b2

Allow exit_point duplication, when the deoptimization info differs be…

b189b91

…cause of spilling

Use bound spill slots for CV (once again)

654ae29

Improve error handling

15bbe57

Removed IR submodule

0605bd2

Remove IR submodule from workflows

87b85aa

Embed IR

afc773a

IR commit: 8977307f4e96ee03847d7f2eb809b3080f9ed662

Add .gitignore

a2ea5d2

Fixed according to feedback

1ce6646

Force C saving preserved registers only for HYBRID VM

7e77941

Update IR

943ec88

IR commit: a2f8452b3d35a756cba38924f5c51a48a7207494

cleanup

87666da

Replace ZEND_ASSERT(0) by ZEND_UNREACHABLE()

e64b2b7

Update IR and remove unused IR files

72ade5c

IR commit: 399a38771393c202a741336643118991290b4b1b

Fixed inconsistency between IR code-generation and register-allocation

74e82f2

Update IR

c417f21

IR commit: 86685504274b0c71d9985b3c926dccaca2cacf9b

Update ir_PHI*() according to IR construction API changes

290059a

Fixed 32-bit build

053772d

Update IR

02a9d73

IR commit: d0686408e20cd8c8640e37ed52ab81403a2383cb

Support for ir_TAILCALL() prototype changes

7fba3de

Update IR

2ac2190

IR commit: d72ae866e09d17e879378767aceb91d51894818c

Fixed incorrect extension (ZEXT->SEXT)

44451b0

Fix SSA dominance

487b22f

Update IR

193a03d

IR commit: d60d92516dc5f89b93cdf1df7a54141e83226b07

Fixed support ir_ctx.ret_type

66d40ad

dstogov force-pushed the php-ir branch from c8126b5 to 66d40ad Compare October 20, 2023 15:04

dstogov merged commit caf102d into php:master Oct 23, 2023
7 checks passed

pfustc mentioned this pull request Oct 30, 2023

Assertion `0 && "too long jmp distance"' failed with new JIT on AArch64 #12560

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A new PHP JIT implementation based on IR JIT framework #12079

A new PHP JIT implementation based on IR JIT framework #12079

dstogov commented Aug 29, 2023 •

edited

Loading

nielsdos left a comment •

edited

Loading

dstogov commented Sep 11, 2023

dstogov commented Sep 12, 2023

bukka commented Sep 12, 2023

dstogov commented Sep 12, 2023

homersimpsons commented Sep 13, 2023

dstogov commented Sep 13, 2023

withinboredom left a comment

withinboredom Sep 15, 2023

dstogov Sep 18, 2023

withinboredom Sep 15, 2023

withinboredom Sep 15, 2023

dstogov Sep 18, 2023

withinboredom Sep 15, 2023

dstogov Sep 18, 2023

withinboredom Sep 15, 2023

dstogov Sep 18, 2023

derickr commented Sep 15, 2023

A new PHP JIT implementation based on IR JIT framework #12079

A new PHP JIT implementation based on IR JIT framework #12079

Conversation

dstogov commented Aug 29, 2023 • edited Loading

nielsdos left a comment • edited Loading

Choose a reason for hiding this comment

dstogov commented Sep 11, 2023

dstogov commented Sep 12, 2023

bukka commented Sep 12, 2023

dstogov commented Sep 12, 2023

homersimpsons commented Sep 13, 2023

dstogov commented Sep 13, 2023

withinboredom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derickr commented Sep 15, 2023

dstogov commented Aug 29, 2023 •

edited

Loading

nielsdos left a comment •

edited

Loading